Remove BatchContainer::borrow_as()
#628
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the ability to borrow owned data as a reference type. The borrowing makes a lot of sense when your types are
Tand&T, but less sense when they are a different type (and the requirement blocks some implementations).Generally, going from owned to borrowed data can always be accomplished by moving the data into a container, and imo we should put pressure on this to encourage the use of containers, for a few reasons. The most important is that if we really need data to be hash partitioned and ordered correctly, it should be the same type doing that. Sorting by
TOwnrather thanTRef<'_>has the risk of introducing a footgun, and this is especially possible when the container makes a different call about the partitioning or order (e.g. a hash map orders by a hash of the key, and a btree by the key itself).The PR uses some performance-defective patterns in parts of the code that are not live, e.g. darker corners of dogs^3, and the robin hood hash trace. Both of these should get fixed up before relying on them, but for the moment it should only be a performance caveat.
Notably, in
reduce.rswe actually move from aVec<(Key, Time)>to containers for each, though .. they still derive their order from owned types and the containers (and vector) are ephemeral. If we were to getinterestinginto a container form, we'd have resolved a bit of the ownership issues here (key ownership, at least).